-
-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: create custom logs cleanup function #3610
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just the one comment in the regex ... most likely a no reality issue, but edge case
api/lib/logger.ts
Outdated
// clean up old log files based on maxFiles and maxSize | ||
|
||
const filePathRegExp = new RegExp( | ||
path.basename(settings.filename).replace(/%DATE%/g, '([^.]+)'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
settings.filename contains ".log", right? So the "." should be escaped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Apollon77 in that case [^.]+
means: take everything that is not a dot. So it will stop just before the .log
. This of course could be broken in case %DATE%
contains a dot but 🤷🏼♂️
Example here: https://regex101.com/r/kKW5ox/1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a new version of the regex to make it stronger, it will work for every date format, it just takes everything between the %DATE% prefix/suffix:
Pull Request Test Coverage Report for Build 8007979295Details
💛 - Coveralls |
Ref: #3609